fix(critical): add __dict__ support to RustCoordinator + E2E smoke test#49
Merged
fix(critical): add __dict__ support to RustCoordinator + E2E smoke test#49
Conversation
…tribute assignment PyO3 #[pyclass] types do not support arbitrary attribute assignment by default — they lack __dict__. The old Python ModuleCoordinator was a Python *subclass* of RustCoordinator, giving it __dict__ automatically. When we removed _rust_wrappers.py in v1.2.3 and aliased ModuleCoordinator directly to RustCoordinator, we broke any code that dynamically sets attributes on the coordinator at runtime (e.g. coordinator.session_state, coordinator._tool_dispatch_context, coordinator._arbitrary_attr, etc.). The session_state crash in v1.2.3 and the _tool_dispatch_context crash in v1.2.4 are both symptoms of this same root cause. Fix: add the 'dict' flag to #[pyclass], which enables Python __dict__ on RustCoordinator. Dynamic attribute assignment now works for all attributes — past, present, and future — without needing to enumerate them as explicit Rust fields. The explicit session_state field added in v1.2.4 is retained for backward compatibility; its getter/setter still works correctly alongside __dict__ support. Verified: cargo check -p amplifier-core-py ✓ cargo clippy -p amplifier-core-py ✓ maturin develop ✓ pytest tests/ (549 passed, 1 skipped) ✓ dynamic attr test: _tool_dispatch_context, session_state, _arbitrary_attr ✓
Creates scripts/e2e-smoke-test.sh — a containerized end-to-end smoke test
that validates a locally-built amplifier-core wheel in a clean Docker
environment before any release is tagged.
The test:
1. Builds a wheel from local source via maturin build --release
2. Creates a fresh python:3.12-slim Docker container (isolated, no host pollution)
3. Installs amplifier from GitHub (CLI + foundation from git, core from PyPI)
4. Overrides amplifier-core with the local wheel via uv pip --force-reinstall
5. Runs the real smoke prompt with a real LLM provider (Anthropic):
'Ask recipe author to run one of its example recipes'
6. Detects failures: Python exceptions, AttributeErrors, tool failures, timeouts
7. Reports PASS/FAIL with clear output
8. Cleans up the container automatically (trap EXIT)
This test exercises the full integration path:
CLI startup → session init → LLM call → delegate tool → sub-session spawn
→ recipe tool → bundle file resolution → recipe execution → result return
All three recent incidents (session_state crash v1.2.3, _tool_dispatch_context
crash v1.2.4, version mismatch PyPI publish) would have been caught by this
test before tagging.
Usage:
./scripts/e2e-smoke-test.sh # Build + test (~5 min)
./scripts/e2e-smoke-test.sh --skip-build # Reuse existing dist/ wheel
Reads ANTHROPIC_API_KEY from environment or ~/.amplifier/keys.env.
The wheel was being copied into the container as 'local-core.whl', which is not a valid wheel filename (missing Python tags). uv pip install rejected it with: error: The wheel filename "local-core.whl" is invalid: Must have a Python tag The script continued because the error was swallowed in the bash block, so the override silently failed and the smoke test ran against the previously-installed version (v1.2.2 from PyPI) — masking bugs. Fixes: - Preserve the original wheel filename when copying into the container (e.g. amplifier_core-1.2.4-cp312-cp312-manylinux_2_34_aarch64.whl) - Capture uv pip install output and check it explicitly for success confirmation (grep for 'installed' or 'already satisfied') - After the override, parse the version from the wheel filename and verify amplifier --version reports the expected 'core X.Y.Z' - Fail loudly at each check so the smoke test never runs against the wrong version
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root Cause Fix for v1.2.3/v1.2.4 Incidents
Three incidents in one day traced to a single root cause:
RustCoordinator(PyO3#[pyclass]) lacked__dict__support, breaking all dynamic attribute assignment.The Fix (one line)
The
dictflag enables Python's__dict__on the class, allowing arbitrary attribute assignment — restoring the behavior the old PythonModuleCoordinatorsubclass had implicitly before the v1.2.3 refactor.What This Fixes
TypeError: argument of type 'builtins.RustSession' is not iterablesession_statewas aliased to wrong field (band-aided in v1.2.4)'builtins.RustCoordinator' has no attribute '_tool_dispatch_context'__dict__— any dynamic attr assignment failsThe
#[pyclass(dict)]fix resolves the root cause. Any code in the ecosystem that dynamically sets attributes on the coordinator now works — not just the specific attributes we knew about.E2E Smoke Test Script
New
scripts/e2e-smoke-test.sh— containerized pre-release validation:amplifier run "Ask recipe author to run one of its example recipes"Verification Results
cargo checkcargo clippypytestDO NOT merge this PR yet — waiting for human review after CI passes.